fix(dev): pre-create _data/logs before compose creates it as root - #13194

✅ No Conflicts

No conflict markers, and the branch merges cleanly into its base.

Merge Risk: Minimal · up to 75ce7

Logs are prepared before services start. No identified issue blocks merging.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly describes the main change.
Description check		The description includes the required sections and detailed review steps. It should update the line count and describe the failure handling and recovery hint.
Linked Issues check		[#13082] services_up() creates _data/logs before Docker Compose starts. The later mkdir remains as a safety net. The author reports an ordering test and a syntax check, but not a full Docker run…
Out of Scope Changes check		[#13082] The directory creation and ownership recovery hint address the startup failure. The changes are limited to scripts/development/dev-local.sh.

Full details: Linked Issues check

Explanation

[#13082] services_up() creates _data/logs before Docker Compose starts. The later mkdir remains as a safety net. The author reports an ordering test and a syntax check, but not a full Docker run. The issue excludes make dev-wipe from the fix.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Hey @bharathvemula, thanks for the fix, and for being clear about what you could and couldn't test. The ordering change looks right to me.

One small thing: anyone who already ran the broken version has a root-owned _data, so the new mkdir -p fails in services_up with a bare Permission denied. A hint there would save them a search:

if ! mkdir -p "$REPO_ROOT/_data/logs" 2>/dev/null; then
  warn "_data is owned by root (created by Docker). Run: sudo chown \"\$USER\" _data"
  exit 1
fi

The chown is deliberately non-recursive, since _data/postgres and _data/neo4j need to keep their container owners. Let me know if you disagree.

Addresses @pedrooot's Oct 29 hint for already-broken runs (root-owned _data).

What changed (new head a0570cf, services_up in scripts/development/dev-local.sh): the bare mkdir -p _data/logs is now guarded — on failure it prints: _data is owned by root (created by Docker). Run: sudo chown "$USER" _data, and exits 1. The chown hint is deliberately non-recursive, so _data/postgres and _data/neo4j keep their container owners, and the later mkdir in all_run is untouched as a safety net. One adaptation: the hint uses ${USER:-$(id -un)} because the script runs under set -u, so an unset USER would otherwise abort instead of printing the hint.

Verification: bash -n passes; reproduced the bare Permission denied case and the new hint case as user nobody against a root-owned _data, plus happy-path and non-recursive-chown recovery scenarios (all pass, subdirs untouched). No full make dev run — Docker is absent on this machine, as flagged in the original PR body.

Could you please re-review when you get a chance?

@pedrooot — the root-owned _data hint fix is pushed (new head a0570cf, see comment above). Could you please re-review when you get a chance? A formal re-request from my account was blocked by GitHub permissions, so tagging you here instead.

Hey guys! I'm the one who opened #13082 (the make dev Permission denied on Linux when _data gets created as root by Docker). I'm still pretty new to contributing here, but from what I understood this PR hasn't been tested with a full make dev run since Docker wasn't available, so I gave it a try on my machine (Pop!_OS 22.04, Docker 29.1.3, Compose v5.6.0) at 75ce73d.

Both cases worked for me:

1. Starting with no _data at all: make dev goes all the way through, _data ends up owned by my user and the tmux session is created normally. On master this is exactly where it used to fail.
2. Starting with a root-owned _data left over from master: make dev now stops right away with the hint, before starting the containers or creating the tmux session. After running sudo chown "$USER" _data, make dev works, and _data/postgres, _data/neo4j and _data/valkey kept their container owners. It also skipped the DB bootstrap, which I think means Postgres reused the data from the master run, so the non-recursive chown doesn't seem to touch the database files. I might be reading too much into that though, so feel free to correct me.

One more thing, when I first hit this bug I tried a recursive chown -R on _data and it broke Postgres (could not open file "global/pg_filenode.map": Permission denied), so as far as I can tell keeping the hint non-recursive is the right call.

Thanks for picking this up, happy to test anything else if it helps, and please correct me if none of this makes sense!